- 
          
- 
                Notifications
    You must be signed in to change notification settings 
- Fork 89
          Implement Basic SimpleQuery Handling 
          #505
        
          New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
| Codecov ReportAttention: Patch coverage is  
 
 Additional details and impacted files@@            Coverage Diff             @@
##             main     #505      +/-   ##
==========================================
+ Coverage   62.16%   63.05%   +0.88%     
==========================================
  Files         130      131       +1     
  Lines       10422    10919     +497     
==========================================
+ Hits         6479     6885     +406     
- Misses       3943     4034      +91     
 🚀 New features to boost your workflow:
 | 
| /// Run a simple text-only query on the Postgres server the connection is connected to. | ||
| /// WARNING: This function is not yet API and is incomplete. | ||
| /// The return type will change to another stream. | ||
| /// | ||
| /// - Parameters: | ||
| /// - query: The simple query to run | ||
| /// - logger: The `Logger` to log into for the query | ||
| /// - file: The file, the query was started in. Used for better error reporting. | ||
| /// - line: The line, the query was started in. Used for better error reporting. | ||
| /// - Returns: A ``PostgresRowSequence`` containing the rows the server sent as the query result. | ||
| /// The sequence be discarded. | ||
| @discardableResult | ||
| public func __simpleQuery( | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what will happen to this PR but the name is underscored for now so we can merge this PR as-is and I can then create follow-up PRs.
65b21d6    to
    5961b27      
    Compare
  
    |  | ||
| mutating func dataRowReceived(_ dataRow: DataRow) -> Action { | ||
| switch self.state { | ||
| case .rowDescriptionReceived(let queryContext, let columns): | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happens for queries with actual rows being returned.
|  | ||
| mutating func commandCompletedReceived(_ commandTag: String) -> Action { | ||
| switch self.state { | ||
| case .messagesSent(let context): | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happens for queries that do not return any rows.
| return .succeedQuery(context.promise, with: result) | ||
| } | ||
|  | ||
| case .rowDescriptionReceived(let context, _): | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happens for queries that do query some rows to be returned, but no rows matched the query.
| demandStateMachine.receivedRow(dataRow) | ||
| state = .streaming(columns, demandStateMachine) | ||
| let result = QueryResult(value: .rowDescription(columns), logger: queryContext.logger) | ||
| return .succeedQuery(queryContext.promise, with: result) | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to succeed the query here when we receive the first row and state is RowDescription, since we notice that there are rows to be read.
| return self.avoidingStateMachineCoW { state -> Action in | ||
| state = .commandComplete(commandTag: commandTag) | ||
| let result = QueryResult(value: .noRows(.tag(commandTag)), logger: context.logger) | ||
| return .succeedQuery(context.promise, with: result) | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to succeed the query here when we receive the command-complete row and state is RowDescription, since we notice that there are not rows to be read.
| import Logging | ||
| @testable import PostgresNIO | ||
|  | ||
| class SimpleQueryStateMachineTests: XCTestCase { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly copy-pasted from extended-query state machine, though properly adjusted to simple-query state machine and the way it should behave.
25a69ec    to
    39de666      
    Compare
  
    35f6d6e    to
    26fe020      
    Compare
  
    | I wonder if a better way is to build a dedicated  | 
| @fabianfett i moved the stuff to that, mid-PR. | 
| FWIW we're already using this function on prod and it's been working as it should. | 
Resolves #499
Checklist
SimpleQueryresponse withtextformat.Decoding rows from a simple-query response seems to fail for now, although no-row queries do succeed. I think because as described in Postgres docs, the returned format of data for simple-query istext, and PostgresNIO doesn't properly support that.EDIT: For SimpleQuery only, I removed the line that forced formats to be binary, and things are working now.
I'm not sure how well PostgresNIO supports the text format, but it works for strings and ints.
Not Implemented In This PR
SimpleQuery.